Fix transaction re-signing #554
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes transaction re-signing by properly tracking transaction state changes during the signing process. The change ensures that when a transaction's version or options are modified during the signing process (e.g., for hash signing), these alterations are correctly detected to trigger re-signing.
Key changes:
- Added deep copy tracking of the initial transaction state before modifications
- Enhanced alteration detection to compare initial vs modified transaction properties
- Version bump to 11.2.1
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pyproject.toml | Version bump to 11.2.1 for the bug fix release |
| multiversx_sdk_cli/cli_shared.py | Added transaction state tracking and improved alteration detection logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if initial_tx.version != transaction.version: | ||
| altered = True | ||
|
|
||
| if initial_tx.options != transaction.options: | ||
| altered = True |
There was a problem hiding this comment.
The logic checks for changes between initial_tx and transaction before any explicit alterations are made. This creates a redundant check since at this point in the function, transaction should be identical to initial_tx. Consider moving these checks after the explicit alteration logic (lines 904-910) to properly detect all changes.
andreibancioiu
left a comment
andreibancioiu
left a comment
There was a problem hiding this comment.
Missing CLI test.
|
|
||
| altered = _alter_version_and_options_if_provided( | ||
| args=args, | ||
| initial_tx=initial_tx, |
There was a problem hiding this comment.
Naming inconsistency, tx vs. transaction.
|
|
||
| def _alter_version_and_options_if_provided( | ||
| args: Any, | ||
| initial_tx: Transaction, |
There was a problem hiding this comment.
Naming inconsistency.
3 participants
No description provided.